Skip to content

fix(api): stop leaking wallet address in /policies query string (#525) - #628

Merged
nonsobethel0-dev merged 2 commits into
Parashield-Protocol:mainfrom
springswell:fix/525-no-wallet-in-policies-query
Sep 23, 2026
Merged

nonsobethel0-dev merged 2 commits into
Parashield-Protocol:mainfrom
springswell:fix/525-no-wallet-in-policies-query

Conversation

@springswell

@springswell springswell commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

fetchUserPolicies in src/lib/api.ts sent the connected wallet as a query parameter: GET /policies?wallet=G.... Query strings get written to server and proxy access logs and to browser history, so the user's Stellar address leaked in plaintext.

Why the param can simply be removed

The backend already ignores it. In parashield-backend/src/policy/policy.controller.ts, getMyPolicies only reads page and limit from the query string and resolves the wallet from the authenticated request (req.user?.walletAddress || req.wallet):

#345 — wallet used to come from a client-supplied query param … the JWT wallet was always the one actually used, making the param redundant

The axios client already attaches Authorization: Bearer <token> to every request, so removing the param changes nothing on the backend. It also doesn't need moving to a body or a header.

Changes

Commit 1: fix

  • src/lib/api.ts: fetchUserPolicies() takes no argument and calls GET /policies with no query string. Added a doc comment explaining why.
  • src/hooks/usePolicies.ts: calls fetchUserPolicies(). It still skips fetching while no wallet is connected, and still refetches or aborts when the wallet changes.

Commit 2: tests (src/__tests__/usePolicies.test.tsx)

  • New regression test: fetchUserPolicies is called with no arguments.
  • The existing stale-response test used the wallet argument to track in-flight requests. It now uses the wallet that was active when each call was made. What it checks is unchanged.

Testing

npx vitest run src/__tests__/usePolicies.test.tsx src/hooks/__tests__/usePolicies.test.ts
 Tests  1 failed | 13 passed (14)
  • The one failure is usePolicy > refetch allows manual refresh independent of id changes. It also fails on main without this change, and it tests usePolicy, not the code changed here.
  • src/lib/__tests__/api.test.ts also fails on main before any of these changes (Cannot read properties of undefined (reading 'interceptors')). That is unrelated.
  • tsc --noEmit reports no errors in the changed files.

Out of scope / follow-up

fetchUserPolicies sent the wallet as ?wallet=..., leaking the address
into server/proxy access logs and browser history. The backend already
resolves the wallet from the JWT and ignores this param (backend Parashield-Protocol#345),
so drop it and the now-unused argument; usePolicies still gates the
fetch on a connected wallet.

Closes Parashield-Protocol#525
Add a regression test that fetchUserPolicies is called with no wallet
argument, and key the stale-wallet test's pending requests by the wallet
active at call time instead of the removed argument.

Refs Parashield-Protocol#525
@drips-wave

drips-wave Bot commented Sep 23, 2026

Copy link
Copy Markdown

@springswell Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@netlify

netlify Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

❌ Deploy Preview for boisterous-sunshine-dd4c4c failed.

Name Link
🔨 Latest commit bb9ad91
🔍 Latest deploy log https://app.netlify.com/projects/boisterous-sunshine-dd4c4c/deploys/6ab4213d73334b0008a5e5d6

@nonsobethel0-dev
nonsobethel0-dev merged commit 3e5d502 into Parashield-Protocol:main Sep 23, 2026
0 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants